Fix expiration of action goals when EventsExecutors are used#3018
Fix expiration of action goals when EventsExecutors are used#3018skyegalaxy wants to merge 9 commits intoros2:rollingfrom
Conversation
mjcarroll
left a comment
mjcarroll
left a comment
There was a problem hiding this comment.
Thanks! Overall, looks pretty good and solves the memory leak.
I have a few thoughts/concerns that we should take a look at before merging.
fujitatomoya
left a comment
fujitatomoya
left a comment
There was a problem hiding this comment.
the other major downside for Non EventExecutors is that it now calls execute_check_expired_goals twice on each expiration? once from the timer callback (which is added by this PR) and the another from the waitable's execute() call?
|
Hm, we could add 'rcl_action_server_set_expired_callback' and handle this in the rcl layer to only call the callback if the timer expires etc. |
|
Something else to consider: If this leak's still happening for C++ based EventsExecutors, the coupling of goal expiration to the waitset implies that this could also be happening with the rclpy EventsExecutor. I'm not finding any analogous changes on the rclpy side to expire goals from a timer that isn't still using the waitset to check if it's expired. Also, I've verified that this leak is also present in Kilted and whatever solution we arrive at will ultimately need to be backported. |
Signed-off-by: Skyler Medeiros <skye.galaxy@proton.me>
Signed-off-by: Skyler Medeiros <skye.galaxy@proton.me>
Signed-off-by: Skyler Medeiros <skye.galaxy@proton.me>
Signed-off-by: Skyler Medeiros <skye.galaxy@proton.me>
Signed-off-by: Skyler Medeiros <skye.galaxy@proton.me>
Signed-off-by: Skyler Medeiros <skye.galaxy@proton.me>
Signed-off-by: Skyler Medeiros <skye.galaxy@proton.me>
Signed-off-by: Skyler Medeiros <skye.galaxy@proton.me>
skyegalaxy
force-pushed
the
smedeiros/fix-action-expire-leak
branch
from
94ed627 to
dbec660
Compare
|
I've updated this PR with a new approach from the feedback from prior discussions, and added some rcl-side changes here: ros2/rcl#1295 |
4 participants
Description
In the long actions ram usage section of the benchmarks iRobot shared back in September 2025, there was a memory leak identified in rolling for both the EventsExecutor implementations. I did some deeper profiling and found that it was the result of expired goal results never being cleared out.
This PR introduces changes to register an on_ready callback with the action server at the rcl layer. Events based executors will now be able to call
set_on_ready_callbackforEntityType::Expiredevents, and the callback will be called in the rcl timer layer to enqueue an expired event. This allows for goal expiration to happen through the expected code path oftake_data_by_entity_id->execute.Followup to #2699.
Alternative to #3012
Requires ros2/rcl#1295
Is this user-facing behavior change?
Properly fixes a memory leak in actions where goal handles are stored indefinitely and never expired if using the EventsExecutor.
Did you use Generative AI?
No
Additional Information
EventsExecutor heaptrack allocation profile of irobot benchmark using 1MB payload, before:
after:
